Skip to content

fix(kingbase): escape SQL identifiers and literals in metadata/DDL paths (#1914) - #2197

Merged
openai0229 merged 4 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-kingbase
Jul 29, 2026
Merged

fix(kingbase): escape SQL identifiers and literals in metadata/DDL paths (#1914)#2197
openai0229 merged 4 commits into
OtterMind:mainfrom
HandSonic:fix/sqli2-kingbase

Conversation

@HandSonic

@HandSonic HandSonic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #1914

Summary

Completes the KingBase database-plugin portion of the identifier and literal escaping work.

  • Aligns KingBase identifier processing with the PostgreSQL-derived conditional quoting, reserved-word, case, and always-quote round-trip contracts.
  • Uses unconditional identifier quoting across metadata, DDL, manager, inherited select/count/insert/update/drop, and template paths.
  • Preserves schema qualification, case-only renames, custom type expressions, partition names, and definition casing.
  • Reuses the PostgreSQL structural validation baseline for nested defaults and user-defined or parameterized type expressions while rejecting comments, statement boundaries, unbalanced input, and appended column clauses.
  • Quotes index, constraint, table, schema, column, update, and primary-key names consistently and serializes charset text as an escaped string literal.
  • Expands regression coverage for generated SQL and adversarial metadata names.

Affected surfaces

  • Frontend / Web
  • Backend / API / Storage
  • Database plugin / Driver
  • JCEF / Desktop packaging
  • CI / Build / Release
  • Documentation only

Verification

  • Commands and results:
    • mvn -B -f chat2db-community-server/pom.xml -pl chat2db-community-plugins/chat2db-community-kingbase -am clean test -DskipTests=false -Dmaven.test.skip=false -Dsurefire.failIfNoSpecifiedTests=false
    • Result: BUILD SUCCESS across 9 reactor modules; 184 tests passed in tested modules, including KingBase 19/19, PostgreSQL 49/49, SPI 90/90, and tools 26/26.
    • git diff --check origin/main...HEAD: passed.
  • Manual verification: Source-level generated-SQL review completed. No live KingBase instance was used.
  • UI evidence: N/A

Risk and compatibility

  • Public API or stored data: No public API or stored-data schema change. Uses the existing shared ISQLIdentifierProcessor contract.
  • Database or driver compatibility: Conditional lookup/completion quoting remains PostgreSQL-compatible; metadata-driven generated SQL always quotes raw names. Legal nested defaults and custom or parameterized types are covered by regression tests.
  • Network, privacy, or security: No network or privacy change. Reduces malformed SQL and identifier/literal injection risk in generated metadata, DDL, export, and DML paths.
  • Community / Local / Pro boundary: Community KingBase plugin only; Local and Pro consume the Community plugin behavior without product-specific code changes.
  • Backward compatibility: Ordinary identifiers, defaults, types, and values retain their emitted form; mixed-case names, embedded delimiters, and case-only renames now preserve object identity.

Reviewer map

  • Start here: KingBaseSQLIdentifierProcessor, KingBaseSqlGuards, then KingBaseSqlBuilder, KingBaseMetaData, and KingBaseDBManager.
  • Failure condition: Quote/remove loses raw text, legal defaults/types are rewritten, generated SQL contains raw metadata identifiers, schema qualification is lost, or custom types/partitions are silently changed.
  • Rollback or disable path: Revert this PR; no migration or persisted-data rollback is required.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: Substantial AI-assisted maintainer revision, multi-agent review, and test expansion; all final changes were source-reviewed and verified with the commands above.

HandSonic and others added 4 commits July 29, 2026 17:29
… per maintainer review (OtterMind#1914)

- strengthen KingBaseSQLIdentifierProcessor (SPI ISQLIdentifierProcessor):
  quoteIdentifier always double-quotes with embedded-quote doubling,
  escapeString doubles single quotes; INSTANCE for call sites without
  MetaData access
- KingBaseMetaData returns the processor from getSQLIdentifierProcessor();
  metadata call sites use it (KINGBASE_SQL_IDENTIFIER_PROCESSOR removed)
- DBManager/builder/enums use KingBaseSQLIdentifierProcessor.INSTANCE
- non-escapable validation moved to KingBaseSqlGuards (safe expressions,
  index methods)
- KingBaseSqlEscapes removed; tests migrated (15 green)
@openai0229
openai0229 force-pushed the fix/sqli2-kingbase branch from df0a2e2 to 06ef3c2 Compare July 29, 2026 09:49

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review complete. KingBase identifier/literal escaping, PostgreSQL-derived guards, inherited builder paths, and regression coverage were revised and revalidated on the latest main.

@openai0229
openai0229 merged commit b4a900d into OtterMind:main Jul 29, 2026
19 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Security: 34 SQL Injection Vulnerabilities in Database Plugins

2 participants